UEFI: add support for tasks scheduler and make it default - #5553

You mean like this? https://github.com/tinygo-org/tinygo/blob/dev/targets/avr.json#L20

I'm sorry, if there is something that's making src/internal/task/task_stack_avr.S conditionally included, I'm missing it. Does it not matter if an assembly file is included but not used?

I am wonder why you would need it conditionally included when you can just add to target file?

@deadprogram Ah, I think I see what was confusing me. I thought src/internal/task/task_stack_amd64_windows.S was specific to just the tasks scheduler (and thus, should not be included for any other scheduler). But it's fine if it's included even if we're not using the tasks scheduler?

Here are some further edited comments from automated review.

1. task_stack_amd64_winabi.go is a near-verbatim copy of task_stack_amd64_windows.go

Since both share the same .S file too, I'd delete the new file and update the tags instead:

• task_stack_amd64_windows.go: //go:build scheduler.tasks && amd64 && (windows || uefi)
• task_stack_amd64.go: //go:build scheduler.tasks && amd64 && !windows && !uefi (as in the PR)

That drops 58 lines and, more importantly, means a future fix to the register layout can't be applied to one copy and missed in the other. Renaming the file to task_stack_amd64_winabi.go at that point would be a nice touch, but it's cosmetic.

2. ExtraFiles() in compileopts/config.go

• Putting it in Config.ExtraFiles() rather than in target.go is actually the more correct of the two options, because c.Scheduler() reflects a -scheduler= command-line override, whereas the existing asyncify precedent (compileopts/target.go:229) keys off spec.Scheduler and would silently ignore the flag. So I'd keep it here rather than moving it.
• The simpler alternative is just adding src/internal/task/task_stack_amd64_windows.S to extra-files in targets/uefi-amd64.json unconditionally, next to src/runtime/asm_amd64_windows.S which is already there. With scheduler=none the .S would reference tinygo_task_exit, which doesn't exist. If it links, that's ~5 fewer lines of special-casing in shared code. If it doesn't, the config.go approach is justified and I'd say so in a comment there.

Minor: the c.GOARCH() == "amd64" guard is redundant today (uefi-amd64 is the only uefi target and pins goarch: amd64), but harmless as future-proofing.

3. schedulerSleepCustom busy-waits instead of using the sleep queue

for ticks() < deadline {
gosched()
}

The cooperative scheduler's normal path (addSleepTask + task.Pause(), scheduler_cooperative.go:253) should already work on UEFI because ticks(), nanosecondsToTicks(), and sleepTicks() are all implemented in src/runtime/runtime_uefi.go. If the default path works, this file can go away entirely.

If it does need to stay, with this implementation a sleeping goroutine spins the run queue for the whole duration, so time.Sleep never lets the scheduler go idle. That matches existing UEFI behavior (sleepTicks already spins on CpuPause), so it's not a regression but it does mean waitForEvents/SetWaitForEvents never gets a chance to run during a sleep, which may matter for anyone using that hook. Worth a comment in the file either way explaining why the standard sleep queue isn't used.

Also: the //go:linkname gosched runtime.Gosched is unnecessary since sleep_custom_uefi.go is in package runtime, so it can call Gosched() directly. The linkname adds an indirection with no benefit.

@sparques did you see my further feedback? It would be great to be able to get this into the next release... 😸

@deadprogram working on it now! The fall semester has started up and between that and work, I have not had much free time. But I would also like to get this out in next release!

@deadprogram okay, I think that addressed everything. And I did just test in a vm that using goroutines with the tasks scheduler actually works.

@sparques please see errors in CI such as https://github.com/tinygo-org/tinygo/actions/runs/33082639222/job/98553834651?pr=5553#step:18:67

Thanks for the updates. The changes look good.

Two small points remain.

1. Is the uefi build tag on task_none_uefi.go necessary?

The export of tinygo_task_exit is the correct fix. src/internal/task/task_stack.go has the build tag scheduler.tasks || scheduler.cores, so with -scheduler=none the reference from the assembly file has no definition.

But compileopts/target.go:535 adds task_stack_<arch>.S for all non-wasm targets, and it does this for all schedulers. All -scheduler=none builds have the same reference, and they link correctly today. Most probably --gc-sections removes the section. If UEFI is the only target that fails, can you tell us why? If the cause is not specific to UEFI, remove && uefi and put the export in task_none.go. Then one definition covers all targets.

2. schedulerSleepCustom is no longer in use.

sleep_custom_uefi.go is removed, so schedulerSleepCustom has only one implementation, and it always returns false. #5452 added this hook for this purpose only. You can remove src/runtime/sleep_custom_default.go and the call in scheduler_cooperative.go:300.

It seems like the uefi tag is necessary. Without the file at all, I get a link error for a missing symbol. With the file and uefi tag in place, -scheduler=none works for UEFI, no link errors. Without the uefi tag, if I try to compile targeting something else I get this:

ld.lld: error: duplicate symbol: tinygo_task_exit
>>> defined in /tmp/tinygo767625504/main.o
>>> defined in /home/sparques/.cache/tinygo/obj-a0ab48443817d19a4775605051d32c9314e9d3800943f4b7c6c2a181.bc

I removed the unused schedulerSleepCustom stuff.

Almost there! This last thing is that the PR moves the macos-minimal-sdk submodule backwards by five commits. e7c7215 ("Add TargetConditionals.h") is an ancestor of 5f57dee, which is what origin/dev points at and which dev commit 86d58db ("lib: update macos-minimal-sdk to v0.1.0") set deliberately. Almost certainly a stale submodule working copy that got committed during one of the merges.

You can restore the pointer like this:

git submodule update --init lib/macos-minimal-sdk
git add lib/macos-minimal-sdk
git commit -m "lib: restore macos-minimal-sdk pointer"

Thank you very much for all your work on this @sparques now squash/merging!